Update write load monitor log-levels #136137
Conversation
Change the log-levels in the monitor to use debug for activity and trace for no activity. This supports enabling DEBUG logging in production to show when the write load logic activates. Closes ES-13139
DiannaHohensee
added
>non-issue
:Distributed Coordination/Allocation
|
Pinging @elastic/es-distributed-coordination (Team:Distributed Coordination) |
| } else { | ||
| logger.debug("Not calling reroute because we called reroute recently and there are no new hot spots"); | ||
| logger.debug( | ||
| "Not calling reroute because we called reroute [{}] ago and there are no new hot spots", |
There was a problem hiding this comment.
| "Not calling reroute because we called reroute [{}] ago and there are no new hot spots", | |
| "Not calling reroute because we called reroute [{}] ago or there are no new hot spots", |
There was a problem hiding this comment.
I believe this is and. If either enough time passed or a new hot spot, then the if-statement triggers. So the else-statement is when time has not passed AND there are no new hot-spots.
I tried to make that a bit clearer with the new comment before the if-else statement, but seems still tricky.
| """, | ||
| nodeSummary(nodeIdsExceedingLatencyThreshold), | ||
| state.nodes().size(), | ||
| lastRerouteTimeMillis == 0 ? "never" : TimeValue.timeValueMillis(timeSinceLastRerouteMillis), |
There was a problem hiding this comment.
With never, the log message will be was last called [never] ago which reads rather odd. I suggest we either just use TimeValue.timeValueMillis(timeSinceLastRerouteMillis) similar to how that is used in the else branch. Or adjust the logging message further so it reads something like was never called. Also, it would be great to handle it similarly here and in the else branch.
There was a problem hiding this comment.
Agreed, it doesn't flow. I figured that it was clear, but I can make it better 👍
The else-statement has the advantage of not encountering never because it's logically impossible (haveCalledRerouteRecently is always true in that branch). Doesn't apply in this case.
DiannaHohensee
added
auto-merge-without-approval
3 participants
Change the log-levels in the monitor to use debug for activity and
trace for no activity. This supports enabling DEBUG logging in
production to show when the write load logic activates.
Closes ES-13139